Add support for reading ORAS5 data#2422
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2422 +/- ##
==========================================
+ Coverage 96.19% 96.23% +0.04%
==========================================
Files 272 273 +1
Lines 15957 16132 +175
==========================================
+ Hits 15350 15525 +175
Misses 607 607 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Tried with the example recipe and it looks good, output seems as expected. Fast look through the code looks good. I'll do a more detailed review when I have more time. |
Removed instructions for placing data files in specific subdirectories.
bettina-gier
left a comment
There was a problem hiding this comment.
It runs, results look fine. Might be nice to add a sample config for the filename even if it's in a flat folder.
@alistairsellar Is it possible to do some sort of manual CLA override? Jan signed on the github account but most commits are under a faulty config that can't sign. Otherwise we would have to somehow rebase the commits.
Hi @bettina-gier. There's no way to override the CLA list of signatories if GitHub doesn't think that a commit is linked to a GitHub account. What I would suggest is that if you know for certain that all commits in a PR have been made by people who have signed the CLA (like in this case), then you can temporarily switch off the branch protection that looks at the CLA check. (This aligns with the spirit of why we need the CLA.) And then switch the rule back on again after the PR is merged. Happy to do the switch-off when you're ready to merge, though I expect you've got permissions to change rules? |
|
I personally don't have permissions for that I believe. I'll ask for a quick final tech review before merge! |
bouweandela
left a comment
There was a problem hiding this comment.
Looks good! To wrap it up, could you please add a bit more documentation about the various facets that can be used from the recipe to control the grid and remove code that was copy/pasted from the ICON fix but does not apply to ORAS5?
jmalles
left a comment
There was a problem hiding this comment.
Hopefully, all comments are addressed now. Otherwise, let me know!
Thanks, mostly good now! I found a few more things in the tests that appear to be unused and the documentation could be a bit more clear. |
jmalles
left a comment
There was a problem hiding this comment.
I addressed the comments and hope this is good to go now! 🙂
bouweandela
left a comment
There was a problem hiding this comment.
Thanks, looks good to me now
|
Thank you all, I am happy that this is finally done! 🙂 |
Description
Fix for ORAS5. It includes the option to make the 'irregular' (2d) grid 'unstructured' (1d) and UGRID-compliant. To do so, one needs to add the extra facet 'make_unstructured: true' and/or 'ugrid: true' in the recipe.
The mesh description ('horizontal_grid' in the recipe) needs to be read from a file (oras5_mesh.zip), which is based on a file that can be downloaded here. Since ORAS5 data is on the ORCA025 (Arakawa-C grid), there are different mesh files for tracers (T), and zonal/meridional velocities (u/v).
An example recipe for ORAS5 can be found here.
Some already downloaded data are stored here:
/work/bd1083/b382555/extraobsraw/Tier2/ORAS5/data
(for 3D data in all_levels so far usually only short time periods are downloaded, due to their size)
The grid files are also stored on Levante:
/work/bd1083/b382555/extraobsraw/Tier2/ORAS5/grids
Link to documentation: https://esmvaltool--2422.org.readthedocs.build/projects/ESMValCore/en/2422/quickstart/find_data.html#oras5
Before you get started
Checklist
It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.
To help with the number pull requests: